Repository navigation
fix: keep bootstrapRef across the VM creation request-id patch - #800
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #800 +/- ##
===========================================
+ Coverage 43.96% 54.54% +10.57%
===========================================
Files 18 32 +14
Lines 2593 6296 +3703
===========================================
+ Hits 1140 3434 +2294
- Misses 1417 2546 +1129
- Partials 36 316 +280 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
thunderboltsid
approved these changes
Oct 2, 2026
The request-id patch was applied to rctx.NutanixMachine itself, so the client overwrote it with the API server's response and dropped every in-memory change not yet persisted: spec.bootstrapRef, the finalizer added at the top of reconcileNormal, and the deprecated finalizer removal. Patch a DeepCopy instead so those stay on the live object and are persisted by the deferred patch at the end of the reconcile, which makes the bootstrapRef special case in the baseline unnecessary.
Vedansh074
approved these changes
Oct 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rctx.NutanixMachineitself, so the client overwrote it with the API server's response and dropped every in-memory change not yet persisted:spec.bootstrapRef, the finalizer added at the top ofreconcileNormal, and the deprecated finalizer removal.addGuestCustomizationToVMthen panicked onbootstrapRef.Kindduring the first VM create. The retry succeeded (the request id already existed, so no patch), which is why scale tests logged the panic and the cluster still came up.How has this been tested?
go test ./controllers/(new fake-client test assertsbootstrapRefand the finalizer survive the mint patch; nil-ref guards tested). Both new tests fail without the controller change.bootstrapRef, finalizer, labels and status, and that with the fix the deferred patch persists the finalizer,bootstrapRefand request id.quickstart && !clusterclass, cilium) against a dev PC on bothupstream/mainand this branch and inspected the dumped CAPX manager logs:main: 2Observed a panicinaddGuestCustomizationToVM, one per machine on the first create; VMs created on the retry.spec.bootstrapRefpersisted, and nodes joined.